handle conversion of custom use types for files #4225

elrayle · 2020-01-27T21:16:57Z

NOTE: PR #4215 is working toward similar issues around use types. Both approaches need to be reviewed for compatibility.

Fixes #4203

Co-authored-by: cjcolvar cjcolvard@indiana.edu

Conversion of files with custom use type

Conversion of the 3 primary use types were handled in PR #4055. That made use of the AF association target. That does not work for custom file. This PR includes a generalized approach that works for all relationships.

Refactor: type instead of use

There is no conversion path from pcdm_file.metadata_node.type as used in AF to fiile_metadata.use in FileMetadata.

metadata_node.type is an ActiveTriples::Relation that includes all RDF types for the file, not just the use type. For example...

 [#<RDF::URI:0x3fc9c096fe3c URI:http://fedora.info/definitions/v4/repository#Binary>,
  #<RDF::URI:0x3fc9c096f938 URI:http://fedora.info/definitions/v4/repository#Resource>, 
  #<RDF::URI:0x3fc9c096f234 URI:http://pcdm.org/models#File>, 
  #<RDF::URI:0x3fc9c096fa04 URI:http://www.w3.org/ns/ldp#NonRDFSource>,
  #<RDF::URI:0x3fc9c096fb83 URI:http://pcdm.org/use#OriginalFile>]

The only part of this that is the use is the last one ending in #OriginalFile. We could have search the types for one beginning with http://pcdm.org/use, but there is not a guarantee that a site wouldn't have custom uses defined that do not begin with this.

For backward compatibility, we decided to remove the use attribute from FileMetadata and replace it with type.

To support functionality like #original_file, we added a custom query find_many_file_metadata_by_use(resource:, use:) to FindFileMetadata. Then we defined methods #original_file, #extracted_text, and #thumbnail in Hyrax::FileSet using the new custom query to locate the file with the corresponding use.

Refactor: Centralize definition of 3 primary file use types

In making changes to for this PR, we noticed that there were differing URIs used for the 3 primary file use types (i.e., original_file, extracted_text, and thumbnail). To address this, we centralized the definition of the primary uses in Hyrax::FileSet. All references to the associated use URIs were updated to use Hyrax::FileSet.original_file_use, Hyrax::FileSet.extracted_text_use, and Hyrax::FileSet.thumbnail.

* adjust approach to converting files * add custom query find_file_metadata_by_use which is a valkyrie replacement for af object filter_files_by_type * add helper methods original_file, extracted_text, and thumbnail in FileSet resource Co-authored-by: cjcolvar <cjcolvard@indiana.edu>

Refactor file use: * added methods to Hyrax::FileSet defining which Valkyrie::Vocab::PCDMUse URIs should be used in all cases for the primary uses of original_file, extracted_text, and thumbnail. * updated all uses of Valkyrie::Vocab::PCDMUse URIs for the primary uses to use the methods defined in FileSet Added tests for new methods

Instead of putting the use in a use attributes, conversion from AF File copies over all types as they exist in AF into the types attribute of FileMetadata. To find the use, the :include? method is used to check for a specific use in the types. This avoids data duplication by not having both type and use. And it avoids data integrity issues when we aren’t able to correctly identify the use out of the array of types. Co-authored-by: cjcolvar <cjcolvard@indiana.edu>

…xt, thumbnail 2 changes here… ### Parts of FileSet AF behavior #original_file, #extracted_text, and #thumbnail each return a single PCDM::File, So this now has the methods that get these relatiionships from FileMetadata to return `.first` ### Versioning Versioning was looking through to ActiveFedora through the original_file method defined in wings/hydra/pcdm/pcdm_valkyrization_behavior. Since that was method was removed with the addition of Hyrax::FileSet#original_file, it was no longer getting the lookthrough behavior. The tests related to versioning have been marked pending. Issue #3923 addresses FileMetadata and versioning.

lib/wings/services/custom_queries/find_file_metadata.rb

straleyb · 2020-01-27T21:35:39Z

I looked through this and it looked good to me. Only thing I could find was there maybe a different way to populate an array of values possibly using #each_with_object and populating the new object with the files. But thats more of a style like preference.

https://github.com/samvera/hyrax/pull/4225/files#diff-01ac1642e5ca1afd32dd5eef2a819d22R142

Thats the line I was looking at. If someone else looks at it and finds anything, otherwise I say good to go after my one comment is addressed.

straleyb

Approving this and will look to merge when WIP is removed and small comments are addressed

lib/wings/active_fedora_converter.rb

no-reply · 2020-01-27T23:27:23Z

WIP Needs reconciliation of approaches with PR #4215

let's not block on this. rather i can rebase #4215 on this and we can work to resolve any differences in architecture of approach at that point

lib/wings/active_fedora_converter.rb

app/actors/hyrax/actors/file_actor.rb

app/models/hyrax/file_metadata.rb

no-reply · 2020-01-27T23:42:22Z

app/models/hyrax/file_metadata.rb

@@ -76,19 +76,19 @@ def self.for(file:)
      new(label: file.original_filename,
          original_filename: file.original_filename,
          mime_type: file.content_type,
-          use: file.try(:use) || [::Valkyrie::Vocab::PCDMUse.OriginalFile])
+          type: file.try(:type) || [Hyrax::FileSet.original_file_use])


could we use a Dry::Struct/Types default here? something like attribute :type, ::Valkyrie::Types::Set, default: [Hyrax::FileSet.original_file_use]?

@no-reply This is the only requested change I didn't make. I don't have time to work on this today, so I'd like to punt this requested change to later. I opened issue #4226 to track this request.

app/models/hyrax/file_set.rb

lib/wings/active_fedora_converter.rb

app/models/hyrax/file_set.rb

app/services/hyrax/custom_queries/find_file_metadata.rb

Requested code change to use each_with_object was expanded to include all places where the pattern applied in the effected file. This also includes requested change to case statement.

elrayle and others added 4 commits January 27, 2020 11:41

straleyb reviewed Jan 27, 2020

View reviewed changes

lib/wings/services/custom_queries/find_file_metadata.rb Outdated Show resolved Hide resolved

elrayle changed the title ~~handle conversion of custom use types for files~~ [WIP] handle conversion of custom use types for files Jan 27, 2020

straleyb previously approved these changes Jan 27, 2020

View reviewed changes

elrayle dismissed straleyb’s stale review via 6f8864c January 27, 2020 22:46

straleyb reviewed Jan 27, 2020

View reviewed changes

lib/wings/active_fedora_converter.rb Outdated Show resolved Hide resolved

no-reply reviewed Jan 27, 2020

View reviewed changes

lib/wings/active_fedora_converter.rb Outdated Show resolved Hide resolved

no-reply reviewed Jan 27, 2020

View reviewed changes

app/actors/hyrax/actors/file_actor.rb Outdated Show resolved Hide resolved

no-reply reviewed Jan 27, 2020

View reviewed changes

app/models/hyrax/file_metadata.rb Show resolved Hide resolved

elrayle force-pushed the wings/custom_file_assoc branch from 6f8864c to a0fa358 Compare January 27, 2020 23:38

no-reply reviewed Jan 27, 2020

View reviewed changes

app/services/hyrax/custom_queries/find_file_metadata.rb Show resolved Hide resolved

elrayle added 2 commits January 27, 2020 19:14

prefer use of each_with_object

408ae2e

Requested code change to use each_with_object was expanded to include all places where the pattern applied in the effected file. This also includes requested change to case statement.

use constants for primary file use/type

c8e91a5

elrayle force-pushed the wings/custom_file_assoc branch from a0fa358 to c8e91a5 Compare January 28, 2020 00:27

prefer .map over .each_with_object

fc402da

straleyb approved these changes Jan 28, 2020

View reviewed changes

elrayle changed the title ~~[WIP] handle conversion of custom use types for files~~ handle conversion of custom use types for files Jan 28, 2020

elrayle mentioned this pull request Jan 28, 2020

use a Dry::Struct/Types default for FileMetadata type attribute #4226

Closed

straleyb merged commit 093ac4a into master Jan 28, 2020

straleyb deleted the wings/custom_file_assoc branch January 28, 2020 14:31

This was referenced Jan 29, 2020

inconsistency in relation/use/type #4228

Open

Replacement for file_set.original_file #4233

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

handle conversion of custom use types for files #4225

handle conversion of custom use types for files #4225

elrayle commented Jan 27, 2020 •

edited

Loading

straleyb commented Jan 27, 2020

straleyb left a comment •

edited

Loading

no-reply commented Jan 27, 2020 •

edited

Loading

no-reply Jan 27, 2020

elrayle Jan 28, 2020 •

edited

Loading

handle conversion of custom use types for files #4225

handle conversion of custom use types for files #4225

Conversation

elrayle commented Jan 27, 2020 • edited Loading

Conversion of files with custom use type

Refactor: type instead of use

Refactor: Centralize definition of 3 primary file use types

straleyb commented Jan 27, 2020

straleyb left a comment • edited Loading

Choose a reason for hiding this comment

no-reply commented Jan 27, 2020 • edited Loading

no-reply Jan 27, 2020

Choose a reason for hiding this comment

elrayle Jan 28, 2020 • edited Loading

Choose a reason for hiding this comment

elrayle commented Jan 27, 2020 •

edited

Loading

straleyb left a comment •

edited

Loading

no-reply commented Jan 27, 2020 •

edited

Loading

elrayle Jan 28, 2020 •

edited

Loading